x86: consistently serialize CMOS/RTC accesses on rtc_lock

Since RTC/CMOS accesses aren't atomic, there are possible races
between code paths setting the index register and subsequently
reading/writing the data register. This is supposed to be dealt with
by acquiring rtc_lock, but two places up to now lacked respective
synchronization: Accesses to the EFI time functions and
smpboot_{setup,restore}_warm_reset_vector().

This in turn requires no longer directly passing through guest writes
to the index register, but instead using a machanism similar to that
for PCI config space method 1 accesses.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

diff --git a/xen/arch/x86/efi/runtime.c b/xen/arch/x86/efi/runtime.c
index a79e8c6745d3a88d28c9caa92fe619e6c00cad25..b95aa798ca944a20f121c5c441d84bb6978a09b6 100644 (file)
--- a/xen/arch/x86/efi/runtime.c
+++ b/xen/arch/x86/efi/runtime.c
@@ -2,8 +2,9 @@
 #include <xen/cache.h>
 #include <xen/errno.h>
 #include <xen/guest_access.h>
-#include <xen/time.h>
 #include <xen/irq.h>
+#include <xen/time.h>
+#include <asm/mc146818rtc.h>
 
 DEFINE_XEN_GUEST_HANDLE(CHAR16);
 
@@ -81,9 +82,11 @@ unsigned long efi_get_time(void)
 {
     EFI_TIME time;
     EFI_STATUS status;
-    unsigned long cr3 = efi_rs_enter();
+    unsigned long cr3 = efi_rs_enter(), flags;
 
+    spin_lock_irqsave(&rtc_lock, flags);
     status = efi_rs->GetTime(&time, NULL);
+    spin_unlock_irqrestore(&rtc_lock, flags);
     efi_rs_leave(cr3);
 
     if ( EFI_ERROR(status) )
@@ -224,7 +227,7 @@ static inline EFI_GUID *cast_guid(struct xenpf_efi_guid *guid)
 
 int efi_runtime_call(struct xenpf_efi_runtime_call *op)
 {
-    unsigned long cr3;
+    unsigned long cr3, flags;
     EFI_STATUS status = EFI_NOT_STARTED;
     int rc = 0;
 
@@ -238,7 +241,9 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *op)
             return -EINVAL;
 
         cr3 = efi_rs_enter();
+        spin_lock_irqsave(&rtc_lock, flags);
         status = efi_rs->GetTime(cast_time(&op->u.get_time.time), &caps);
+        spin_unlock_irqrestore(&rtc_lock, flags);
         efi_rs_leave(cr3);
 
         if ( !EFI_ERROR(status) )
@@ -256,7 +261,9 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *op)
             return -EINVAL;
 
         cr3 = efi_rs_enter();
+        spin_lock_irqsave(&rtc_lock, flags);
         status = efi_rs->SetTime(cast_time(&op->u.set_time));
+        spin_unlock_irqrestore(&rtc_lock, flags);
         efi_rs_leave(cr3);
         break;
 
@@ -268,8 +275,10 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *op)
             return -EINVAL;
 
         cr3 = efi_rs_enter();
+        spin_lock_irqsave(&rtc_lock, flags);
         status = efi_rs->GetWakeupTime(&enabled, &pending,
                                        cast_time(&op->u.get_wakeup_time));
+        spin_unlock_irqrestore(&rtc_lock, flags);
         efi_rs_leave(cr3);
 
         if ( !EFI_ERROR(status) )
@@ -288,12 +297,14 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *op)
             return -EINVAL;
 
         cr3 = efi_rs_enter();
+        spin_lock_irqsave(&rtc_lock, flags);
         status = efi_rs->SetWakeupTime(!!(op->misc &
                                           XEN_EFI_SET_WAKEUP_TIME_ENABLE),
                                        (op->misc &
                                         XEN_EFI_SET_WAKEUP_TIME_ENABLE_ONLY) ?
                                        NULL :
                                        cast_time(&op->u.set_wakeup_time));
+        spin_unlock_irqrestore(&rtc_lock, flags);
         efi_rs_leave(cr3);
 
         op->misc = 0;

diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c
index 2f8c957d759d83e7b97a0fc59bc08184dc8720c6..f3a298fefc6d9d140983c8c62aa2e0b0d1e894c7 100644 (file)
--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -502,18 +502,10 @@ static void hpet_detach_channel(unsigned int cpu,
 
 #include <asm/mc146818rtc.h>
 
-void (*__read_mostly pv_rtc_handler)(unsigned int port, uint8_t value);
+void (*__read_mostly pv_rtc_handler)(uint8_t index, uint8_t value);
 
-static void handle_rtc_once(unsigned int port, uint8_t value)
+static void handle_rtc_once(uint8_t index, uint8_t value)
 {
-    static int index;
-
-    if ( port == 0x70 )
-    {
-        index = value;
-        return;
-    }
-
     if ( index != RTC_REG_B )
         return;
     

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 375f13c89e8766ef7b027c015bfbf999d6673393..183ce91b74f791933c00d932d19b70949a5f6795 100644 (file)
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -69,6 +69,7 @@
 #include <asm/hypercall.h>
 #include <asm/mce.h>
 #include <asm/apic.h>
+#include <asm/mc146818rtc.h>
 #include <asm/hpet.h>
 #include <public/arch-x86/cpuid.h>
 
@@ -1640,6 +1641,10 @@ static int admin_io_okay(
     if ( (port == 0xcf8) && (bytes == 4) )
         return 0;
 
+    /* We also never permit direct access to the RTC/CMOS registers. */
+    if ( ((port & ~1) == RTC_PORT(0)) )
+        return 0;
+
     return ioports_access_permitted(v->domain, port, port + bytes - 1);
 }
 
@@ -1669,6 +1674,21 @@ static uint32_t guest_io_read(
         {
             sub_data = pv_pit_handler(port, 0, 0);
         }
+        else if ( (port == RTC_PORT(0)) )
+        {
+            sub_data = v->domain->arch.cmos_idx;
+        }
+        else if ( (port == RTC_PORT(1)) &&
+                  ioports_access_permitted(v->domain, RTC_PORT(0),
+                                           RTC_PORT(1)) )
+        {
+            unsigned long flags;
+
+            spin_lock_irqsave(&rtc_lock, flags);
+            outb(v->domain->arch.cmos_idx & 0x7f, RTC_PORT(0));
+            sub_data = inb(RTC_PORT(1));
+            spin_unlock_irqrestore(&rtc_lock, flags);
+        }
         else if ( (port == 0xcf8) && (bytes == 4) )
         {
             size = 4;
@@ -1702,8 +1722,6 @@ static void guest_io_write(
     {
         switch ( bytes ) {
         case 1:
-            if ( ((port == 0x70) || (port == 0x71)) && pv_rtc_handler )
-                pv_rtc_handler(port, (uint8_t)data);
             outb((uint8_t)data, port);
             if ( pv_post_outb_hook )
                 pv_post_outb_hook(port, (uint8_t)data);
@@ -1726,6 +1744,23 @@ static void guest_io_write(
         {
             pv_pit_handler(port, (uint8_t)data, 1);
         }
+        else if ( (port == RTC_PORT(0)) )
+        {
+            v->domain->arch.cmos_idx = data;
+        }
+        else if ( (port == RTC_PORT(1)) &&
+                  ioports_access_permitted(v->domain, RTC_PORT(0),
+                                           RTC_PORT(1)) )
+        {
+            unsigned long flags;
+
+            if ( pv_rtc_handler )
+                pv_rtc_handler(v->domain->arch.cmos_idx & 0x7f, data);
+            spin_lock_irqsave(&rtc_lock, flags);
+            outb(v->domain->arch.cmos_idx & 0x7f, RTC_PORT(0));
+            outb(data, RTC_PORT(1));
+            spin_unlock_irqrestore(&rtc_lock, flags);
+        }
         else if ( (port == 0xcf8) && (bytes == 4) )
         {
             size = 4;
@@ -2091,10 +2126,6 @@ static int emulate_privileged_op(struct cpu_user_regs *regs)
             goto fail;
         if ( admin_io_okay(port, op_bytes, v, regs) )
         {
-            if ( (op_bytes == 1) &&
-                 ((port == 0x71) || (port == 0x70)) &&
-                 pv_rtc_handler )
-                pv_rtc_handler(port, regs->eax);
             io_emul(regs);            
             if ( (op_bytes == 1) && pv_post_outb_hook )
                 pv_post_outb_hook(port, regs->eax);

diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 382e2dbcb2cb38d79ca8396beabafd4f97d7ed40..3e4408a0b94697762d710b59470a6c3b89d95a81 100644 (file)
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -258,6 +258,7 @@ struct arch_domain
     /* I/O-port admin-specified access capabilities. */
     struct rangeset *ioport_caps;
     uint32_t pci_cf8;
+    uint8_t cmos_idx;
 
     struct list_head pdev_list;
 

diff --git a/xen/include/asm-x86/hpet.h b/xen/include/asm-x86/hpet.h
index 550958cf29d66288ccc35ff13c5ff663352b0865..fe9f9b6ded1fc73997b15e58dac0a350c46fde30 100644 (file)
--- a/xen/include/asm-x86/hpet.h
+++ b/xen/include/asm-x86/hpet.h
@@ -74,6 +74,6 @@ void hpet_broadcast_exit(void);
 int hpet_broadcast_is_available(void);
 void hpet_disable_legacy_broadcast(void);
 
-extern void (*pv_rtc_handler)(unsigned int port, uint8_t value);
+extern void (*pv_rtc_handler)(uint8_t reg, uint8_t value);
 
 #endif /* __X86_HPET_H__ */

diff --git a/xen/include/asm-x86/mach-default/smpboot_hooks.h b/xen/include/asm-x86/mach-default/smpboot_hooks.h
index a70d279912cf28d1f249246ec0b2c255be4946f9..14e1ee52a37d3bd779f9933047724124bfe6fd2d 100644 (file)
--- a/xen/include/asm-x86/mach-default/smpboot_hooks.h
+++ b/xen/include/asm-x86/mach-default/smpboot_hooks.h
@@ -3,7 +3,11 @@
 
 static inline void smpboot_setup_warm_reset_vector(unsigned long start_eip)
 {
+       unsigned long flags;
+
+       spin_lock_irqsave(&rtc_lock, flags);
        CMOS_WRITE(0xa, 0xf);
+       spin_unlock_irqrestore(&rtc_lock, flags);
        flush_tlb_local();
        Dprintk("1.\n");
        *((volatile unsigned short *) TRAMPOLINE_HIGH) = start_eip >> 4;
@@ -14,6 +18,8 @@ static inline void smpboot_setup_warm_reset_vector(unsigned long start_eip)
 
 static inline void smpboot_restore_warm_reset_vector(void)
 {
+       unsigned long flags;
+
        /*
         * Install writable page 0 entry to set BIOS data area.
         */
@@ -23,7 +29,9 @@ static inline void smpboot_restore_warm_reset_vector(void)
         * Paranoid:  Set warm reset code and vector here back
         * to default values.
         */
+       spin_lock_irqsave(&rtc_lock, flags);
        CMOS_WRITE(0, 0xf);
+       spin_unlock_irqrestore(&rtc_lock, flags);
 
        *((volatile int *) maddr_to_virt(0x467)) = 0;
 }